Deprecate table client scan_query in favor of QueryService - #893
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #893 +/- ##
==========================================
+ Coverage 82.61% 82.66% +0.04%
==========================================
Files 99 99
Lines 12945 12951 +6
Branches 1268 1268
==========================================
+ Hits 10695 10706 +11
+ Misses 1799 1793 -6
- Partials 451 452 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
🟡 Changes recommended
The new docstrings use an invalid Sphinx .. deprecated:: directive form (missing required version) and the new warning behavior lacks accompanying tests to prevent regressions.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR deprecates the legacy Table client scan query APIs by emitting DeprecationWarning at runtime and documenting the deprecation, steering users toward QueryService (QuerySessionPool) while keeping existing functionality intact.
Changes:
- Added
DeprecationWarningemission forTableClient.scan_query,TableClient.async_scan_query, andydb.aio.TableClient.scan_query. - Introduced a shared deprecation message constant used across sync/async implementations.
- Added a top-of-file
CHANGELOG.mdentry describing the deprecation.
File summaries
| File | Description |
|---|---|
ydb/table.py |
Adds shared deprecation message and emits DeprecationWarning from the sync and “async_*” TableClient scan query methods; adds deprecation docstrings. |
ydb/aio/table.py |
Emits DeprecationWarning from the async table client scan_query and reuses the shared deprecation message. |
CHANGELOG.md |
Documents the deprecation as user-facing behavior. |
Review details
Suppressed comments (2)
ydb/table.py:1225
- Same issue here:
.. deprecated::without a version argument is invalid in Sphinx and may break autodoc builds. Prefer a plain-text deprecation note (or add an explicit version).
"""
.. deprecated::
Use QueryService (:class:`ydb.QuerySessionPool`) instead.
"""
ydb/table.py:1291
- Same Sphinx directive issue:
.. deprecated::without a version argument is invalid and can fail doc generation. Replace with plain text or include a version.
"""
.. deprecated::
Use QueryService (:class:`ydb.QuerySessionPool`) instead.
"""
- Files reviewed: 3/3 changed files
- Comments generated: 5
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
# Conflicts: # CHANGELOG.md # ydb/table.py # ydb/table_test.py
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The newly added async unit test lacks the repo’s async test marker (@pytest.mark.asyncio), which is likely to break or skip the test run.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (2)
Resolved since last review (5)
ydb.QuerySessionPool,…DeprecationWarning). The repo has unit…deprecateddirective requires a version argument;.. deprecated::with no…deprecateddirective needs a version argument; using it without one in an autodoc-parsed…
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
Deprecation behavior is implemented consistently across sync/async clients, covered by focused tests, and reflected in docs and the changelog without introducing breaking changes.
Review effort: Lite
Findings: None
Resolved since last review (2)
TableClient.scan_query, TableClient.async_scan_query, and ydb.aio.TableClient.scan_query now emit DeprecationWarning and direct users to QueryService through the corresponding QuerySessionPool. The methods remain functional; the documentation now points new code to QueryService.